Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Adding logic to check module dependencies at module loading time #3443

Closed

Conversation

Ocramius
Copy link
Member

Provides implementation for #3427

@Ocramius
Copy link
Member Author

If this one is okay (from the logic perspective/coding), please let me know and don't merge until I wrote the docs for it (I noticed it's the only way to get me to write docs =_= )

@marc-mabe
Copy link
Member

What if two modules depend on each other?

@Ocramius
Copy link
Member Author

@marc-mabe makes no real sense since modules have a loading order. The feature can be turned off at any time btw. It's just a safety check before letting the spaceship starts the engines.

@marc-mabe
Copy link
Member

@Ocramius As it's only a check does it make sense to check it after loading.
Circular dependencies could make sense on closed environments like company internal modules.

@Ocramius
Copy link
Member Author

@marc-mabe loading (including init triggers) may depend on other modules anyway. That's why I do it so early. Even tempted to handle it before autoloading eventually.

@marc-mabe
Copy link
Member

@Ocramius Is the loading process not known the list of modules for loading - isn't it?
So the loading order could be changed automatically to load depended modules first?
To support circular dependencies the module itself need to say which dependencies are required for initialization and which are required after the loading process finished.

Does this make sense ?

@Ocramius
Copy link
Member Author

@marc-mabe no, my point is exactly to AVOID any kind of magic here. It's just a check. If you know what you're doing, you can turn it off and handle dependencies as you wish.

This is just a very important thing for people who may (example) install ZfcUserDoctrineORM and don't enable ZfcUser, and in turn forget to enable ZfcBase. This is a self-documenting exception that helps beginners in getting started, nothing more. No changing of anything :)

And yes, it would just help discovering circular dependencies. Doesn't mean it has to solve 'em :)

Ocramius added a commit to Ocramius/zf2-documentation that referenced this pull request Jan 16, 2013
Adding description of how the ModuleDependencyIndicatorInterface works
and how the listener associated to it behaves
@marc-mabe
Copy link
Member

@Ocramius I'm on you. Because it's a development feature and can be disabled - circular dependencies aren't a must-have and too much magic isn't good.

@Ocramius
Copy link
Member Author

AFAIK, this is complete

* @package Zend_ModuleManager
* @subpackage Exception
*/
class MissingDependencyModuleException extends \RuntimeException implements ExceptionInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should extend Zend\ModuleManager\Exception\RuntimeException, not the base exception class.

@prolic
Copy link
Contributor

prolic commented Jan 17, 2013

Implementation looks straightforward. I like it. Good work!

@Ocramius
Copy link
Member Author

Done, rebased :shipit:

@weierophinney
Copy link
Member

Looks good. The only issue I have is similar to what @prolic says: getDependencyModules doesn't read well, and is hard to understand. May I suggest getModuleDependencies()? If you agree, I can even make such a change at merge time.

@weierophinney
Copy link
Member

Merged -- see a087928

weierophinney added a commit to zendframework/zend-modulemanager that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-modulemanager that referenced this pull request May 15, 2015
Close zendframework/zendframework#3443

Modified original to rename getDependencyModules to getModuleDependencies.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants